Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Render a link to source repo #22

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Render a link to source repo #22

wants to merge 3 commits into from

Conversation

paul121
Copy link
Member

@paul121 paul121 commented Dec 13, 2021

This needs to be ironed out a bit more, but opening a PR to see if this is going in the right direction. See #21

I noticed that the gatsby-source-git plugin adds a gitRemote field with different bits of data, including a webLink. I think that could work well here! As mentioned in their docs, this might help to implement an "edit on github" link as well: https://www.gatsbyjs.com/plugins/gatsby-source-git/

For demonstration I put the source link above the table of contents. This seems like a natural area to include the source link (and maybe the source version??) but open to ideas! This mostly fulfilled the exercise of "I want to put a link here, how do I do that" and to help learn how some of these things are working

I'm also not sure how this React markup works.. I'm sure there's a better way to render a stylized link here... but I guess maybe we want the Github icon prefixed as well... so perhaps this could be its own component? Maybe one already exists?

@netlify
Copy link

netlify bot commented Dec 13, 2021

✔️ Deploy Preview for gracious-brattain-bdd606 ready!

🔨 Explore the source changes: 6842546

🔍 Inspect the deploy log: https://app.netlify.com/sites/gracious-brattain-bdd606/deploys/61b6f2547ab16a0007b989cf

😎 Browse the preview: https://deploy-preview-22--gracious-brattain-bdd606.netlify.app

@jgaehring
Copy link
Member

Awesome! Love how you banged this out! 🥇

And sorry, I feel like I've been peppering comments about this PR in other issues w/o fully addressing it here, so let me pull some of those together...

First, from #12 (comment):

Currently, there is no https://docs.farmos.org/development page, but that might actually be a good place to add links to the repositories for the source code itself, as mentioned in #21 and #22. Then we could use the webLinks from gatsby-source-git that you added for links to editing the actual documentation pages, like this.

To which you responded:

How would this look in the menu? /development/introduction? I think that would be a nice addition regardless, but would also be good to link to the client libraries from within their own pages. If a user uses the menu to go directly to "farmOS.js Docs" you should be able to get to the source repo without going back to /development/introduction.

And I back:

I guess this goes together with some ideas I've been stewing on WRT #22 as well (sorry I haven't responded there, might take me a little while today). I'm thinking it might be better to add it straight to the navigation menu on the left, with the "open-in-new" icon. I feel like it fits in better there than among the table of contents, since it links to another page, rather than a section of the page. And I feel like that gives it more prominence, as well.

So maybe we could pick up that thread here.

Apart from that, I was going to suggest moving the sourceLink out of the GraphQL query variables in createPages and into the query in docs-pages.js itself, but if we do move this link to the main nav, well, that would change things. We'd probably keep it in gatsby-node.js but move it to the onPostBootstrap hook instead, which is where the navigation tree gets built up. From there, we could continue to use a GraphQL query, but frankly, I think we should just pull in git-url-parse, which gatsby-source-git is already using under the hood anyways. That way we cut out the middleman and don't need to fiddle with more GraphQL queries.

Lemme know what you think about all that. I'm gonna try plowing ahead with #12 in the meantime, and will probably be AFK most of tomorrow, so I may not be super responsive, but thanks for taking the charge on this! Huzzah!

@paul121
Copy link
Member Author

paul121 commented Dec 13, 2021

I'm thinking it might be better to add it straight to the navigation menu on the left, with the "open-in-new" icon.

Ah yep, I like this is idea! Not that it's too important, but I realized the table of contents nav is hidden on mobile anyways.

As far as implementation goes I think the latter idea makes sense.. but to clarify, by "pulling in the plugin" does that mean it will add these fields itself? Then they will be available elsewhere without our doing? I'm starting to get an idea how this Gatsby thing works :D

@jgaehring
Copy link
Member

I realized the table of contents nav is hidden on mobile anyways.

Ah yea, good point.

by "pulling in the plugin" does that mean it will add these fields itself?

Oh no, nothing so fancy. git-url-parse is just an npm package, so you can just import it like any module, instantiate a parsed URL using the remote property we get from sourceRepos, then call .toString() on it to get the same as webLink. Although frankly, we could also just use some regex and get the same effect:

const webLink = config.remote.replace(/\.git$/, '');

I hesitated to suggest that before b/c it may not be as rigorous and wouldn't catch for, say, an ssh:// scheme, but I don't think we actually have to worry about that, right? Maybe just easier to skip git-url-parse. But if we do want to use it, it's already in the dependency graph so why not. Up to you.

This was referenced Dec 18, 2021
@mstenta
Copy link
Member

mstenta commented Jan 15, 2024

It occurs to me that we still don't have a link to GitHub repositories on farmOS.org (3 years after this issue was opened). 😅

How about we rename this PR to "Automatically render a link to source repo" and we open a new PR that manually adds some links somewhere? Maybe just on the homepage? :-)

We had a similar complaint recently from someone who pointed out that they were frustrated that the homepage didn't have any simple "Download" button or composer command to run to get farmOS. Of course, there's some complexity there in that we have a few different "recommended" ways to install farmOS, but the point was well taken...

I'll throw a quick PR together that just adds quick links to the GitHub repos to the homepage, along with a link to the "Install" guide.

@mstenta
Copy link
Member

mstenta commented Jan 15, 2024

#87

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants